types(reactivity): disallow using functions with arguments as a computed getter (fix #5692) #5695
Conversation
✅ Deploy Preview for vuejs-coverage ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for vue-sfc-playground ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for vue-next-template-explorer ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Unfortunately the relaxed arguments type seems necessary for the type tests to pass. |
|
I relaxed the type back to what it was but only applied it for computed options. All tests should be passing now. |
packages/reactivity/src/computed.ts
Outdated
| @@ -15,14 +15,20 @@ export interface WritableComputedRef<T> extends Ref<T> { | |||
| readonly effect: ReactiveEffect<T> | |||
| } | |||
|
|
|||
| export type ComputedGetter<T> = (...args: any[]) => T | |||
| export type ComputedGetter<T> = () => T | |||
| export type ComputedGetterWithVModel<T> = (...args: any[]) => T | |||
There was a problem hiding this comment.
Let's rename this to ComputedGetterWithInstance
unshame
force-pushed
the
fix/computed-type-safety
branch
from
656e0f2 to
931f0b8
Compare
|
Any chance to get this merged? |
packages/reactivity/src/computed.ts
Outdated
| export type ComputedSetter<T> = (v: T) => void | ||
|
|
||
| export interface WritableComputedOptions<T> { | ||
| get: ComputedGetter<T> | ||
| set: ComputedSetter<T> | ||
| } | ||
|
|
||
| export interface WritableComputedOptionsWithVModel<T> { |
There was a problem hiding this comment.
Sorry I wasn't more explicit but this needs to be renamed to ...WithInstance for consistency
…k in options API Add special versions of ComputedGetter and WritableComputedOptions that accept view model as argument
Relax type of ComputedGetterWithVModel
unshame
force-pushed
the
fix/computed-type-safety
branch
from
931f0b8 to
363d6e9
Compare
|
@yyx990803 any chance to get this merged? |
|
I was wondering whether this PR is still needed since the changes in #9497? That PR introduced the |
See #5692
This came up during refactoring: I had a computed that was using a vuex getter.
Then I refactored getItems to require a parent id.
And typescript didn't notify me that getItems was used incorrectly.
I ended up banning passing functions into
computeddirectly withno-restricted-syntaxeslint rule.But I don't see why the type for ComputedGetter is declared as it is. Surely there can't be a case where doing the following is valid: